[Feat] Add job posting sequence analysis lookup (#27)#89
Conversation
📝 WalkthroughWalkthroughThe PR refactors analysis retrieval from a sequence-optional path to a dual-path system: direct mockApplyId lookup and job-posting-sequence lookup. The existing AnalysisController endpoint is simplified by removing the optional sequence parameter. A new JobPostingAnalysisController exposes job-posting-based analysis retrieval. AnalysisService is updated to support both paths via a new method and updated sequence resolver using JobPostingService dependency. ChangesAnalysis Endpoint and Service Refactoring
Sequence DiagramsequenceDiagram
participant Client
participant AnalysisController as AnalysisController<br/>(MockApply path)
participant JobPostingAnalysisController
participant AnalysisService
participant JobPostingService
Client->>AnalysisController: GET /api/mock-applies/{mockApplyId}/analysis
AnalysisController->>AnalysisService: getAnalysis(user, mockApplyId)
AnalysisService->>AnalysisService: Load Analysis + QuestionAnalysis
AnalysisService-->>AnalysisController: AnalysisResponse
AnalysisController-->>Client: ApiResponse<AnalysisResponse>
Client->>JobPostingAnalysisController: GET /api/job-postings/{jobPostingId}/analysis?sequence=N
JobPostingAnalysisController->>AnalysisService: getAnalysisByJobPostingSequence(user, jobPostingId, N)
AnalysisService->>JobPostingService: Resolve JobPosting
JobPostingService-->>AnalysisService: JobPosting entity
AnalysisService->>AnalysisService: Resolve MockApply by sequence
AnalysisService->>AnalysisService: Load Analysis + QuestionAnalysis
AnalysisService-->>JobPostingAnalysisController: AnalysisResponse
JobPostingAnalysisController-->>Client: ApiResponse<AnalysisResponse>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/AnalysisController.java`:
- Around line 40-47: The GET handler AnalysisController#getAnalysis currently
matches requests that include a legacy sequence query param and ignores it;
update the mapping to explicitly reject requests that include sequence by adding
params = "!sequence" to the `@GetMapping` on getAnalysis (e.g., `@GetMapping`(params
= "!sequence")), so requests with ?sequence=... won't match this handler and
will fail fast; keep the method signature and service call unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a88e5382-00dd-4ce6-99f2-9ec0e46f3da9
📒 Files selected for processing (4)
src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/AnalysisController.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/controller/JobPostingAnalysisController.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisService.javasrc/test/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisServiceTest.java
| @GetMapping | ||
| public ApiResponse<AnalysisResponse> getAnalysis( | ||
| @AuthenticationPrincipal UserDetailsImpl userDetails, | ||
| @PathVariable Long mockApplyId, | ||
| @RequestParam(required = false) Integer sequence | ||
| @PathVariable Long mockApplyId | ||
| ) { | ||
| return ApiResponse.onSuccess( | ||
| "자소서 분석 결과 조회에 성공했습니다.", | ||
| analysisService.getAnalysis(getAuthenticatedUser(userDetails), mockApplyId, sequence) | ||
| analysisService.getAnalysis(getAuthenticatedUser(userDetails), mockApplyId) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Spring MVC, if a controller method uses @GetMappingand does not declare@RequestParam("sequence"), does a request to the same path with ?sequence=2still match and ignore the extra query parameter by default? Also, does@GetMapping(params = "!sequence") prevent that request from matching?
💡 Result:
Yes: by default, if a controller method is mapped only with @GetMapping (matching by path and HTTP method) and it does NOT declare any request-parameter conditions via @RequestMapping(params=...) / @GetMapping(params=...), then an extra query parameter like?sequence=2 does not prevent the request from matching. Spring will still route to the handler method; @RequestParam affects only whether you can bind a specific parameter into a method argument, not whether unmapped extra query parameters block request mapping. [1][2] No: @GetMapping(params = "!sequence") does not “ignore”?sequence=2; instead it adds a request-mapping condition that requires the parameter sequence to be absent. Spring’s request-mapping “params” condition supports negation with the! form ("!myParam" means the specified parameter is not supposed to be present in the request). Therefore, a request to the same path with sequence=2 will NOT match a mapping declared with params="!sequence". [1][3][4] Example outcomes: - @GetMapping(path="/x") with no params condition: /x?sequence=2 still matches. - @GetMapping(path="/x", params="!sequence"): /x?sequence=2 does not match; only /x with no sequence parameter matches.
Citations:
- 1: https://docs.spring.io/spring-framework/reference/6.2-SNAPSHOT/web/webmvc/mvc-controller/ann-requestmapping.html
- 2: https://docs.spring.io/spring-framework/docs/5.3.28/javadoc-api/org/springframework/web/bind/annotation/RequestParam.html
- 3: https://docs.spring.io/spring-framework/docs/6.0.5/javadoc-api/org/springframework/web/bind/annotation/RequestMapping.html
- 4: https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced controllers
ls -R src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/ 2>/dev/null || true
# Show the relevant portions of AnalysisController
sed -n '1,200p' src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/AnalysisController.java | nl -ba | sed -n '1,220p'
# Search for sequence usage in the analysis controller package
rg -n "sequence" src/main/java/com/jobdri/jobdri_api/domain/analysis/controller -S || true
# Show JobPostingAnalysisController mapping and method(s) mentioning sequence
sed -n '1,240p' src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/JobPostingAnalysisController.java | nl -ba | sed -n '1,260p'Repository: JobDri-Developer/BackEnd
Length of output: 276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant parts of AnalysisController
sed -n '1,220p' src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/AnalysisController.java
# Search for "sequence" usage in the analysis controller package
rg -n "sequence" src/main/java/com/jobdri/jobdri_api/domain/analysis/controller -S || true
# Show relevant parts of JobPostingAnalysisController
sed -n '1,260p' src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/JobPostingAnalysisController.javaRepository: JobDri-Developer/BackEnd
Length of output: 5211
Reject legacy sequence query param on /api/mock-applies/{mockApplyId}/analysis GET to avoid silent wrong results
AnalysisController#getAnalysis maps GET /api/mock-applies/{mockApplyId}/analysis with @GetMapping but does not declare a sequence request parameter (no @RequestParam("sequence") and no params condition). A request to /api/mock-applies/{mockApplyId}/analysis?sequence=N will still match this handler, and sequence will be ignored, returning results based only on mockApplyId—silently breaking the expected “sequence-based” behavior clients may be using.
GET /api/job-postings/{jobPostingId}/analysis?sequence=N
exists in JobPostingAnalysisController, so fail fast on the legacy route by rejecting requests that include sequence.
🧭 Minimal guard to fail fast on legacy `sequence` calls
- `@GetMapping`
+ `@GetMapping`(params = "!sequence")
public ApiResponse<AnalysisResponse> getAnalysis(
`@AuthenticationPrincipal` UserDetailsImpl userDetails,
`@PathVariable` Long mockApplyId
) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/jobdri/jobdri_api/domain/analysis/controller/AnalysisController.java`
around lines 40 - 47, The GET handler AnalysisController#getAnalysis currently
matches requests that include a legacy sequence query param and ignores it;
update the mapping to explicitly reject requests that include sequence by adding
params = "!sequence" to the `@GetMapping` on getAnalysis (e.g., `@GetMapping`(params
= "!sequence")), so requests with ?sequence=... won't match this handler and
will fail fast; keep the method signature and service call unchanged.
1 participant
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#27]
Summary by CodeRabbit
New Features
Refactor